Allowing on RedirectURIs the usage of regexp due to issue #448. - #4930
Open
SGT911 wants to merge 3 commits into
Open
Allowing on RedirectURIs the usage of regexp due to issue #448.#4930SGT911 wants to merge 3 commits into
SGT911 wants to merge 3 commits into
Conversation
…p#448. Adding new flags for conditional logic and allowing back compatibility of the feature Signed-off-by: Sebastian Gaviria Tangarife <sgt.911@outlook.com>
…atch abuse Signed-off-by: Sebastian Gaviria Tangarife <sgt.911@outlook.com>
Signed-off-by: Sebastian Gaviria Tangarife <sgt.911@outlook.com>
|
hey @sagikazarmark who can review such changes? |
Member
|
@sergiofteixeira thanks for the ping. Added to my list, but I'm not in front of a computer right now. Ping me next week please if I don't respond. |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds opt-in regular-expression matching for static-client redirect URIs while retaining literal matching by default.
Changes:
- Adds per-client regexp and wildcard flags.
- Implements startup validation, runtime matching, and tests.
- Qualifies Docker image references with registry names.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
storage/storage.go |
Defines the new client flags. |
server/authflow/request.go |
Implements regexp redirect matching. |
server/authflow/request_test.go |
Tests regexp and wildcard behavior. |
cmd/dex/serve.go |
Validates configured expressions at startup. |
Dockerfile |
Qualifies builder image references. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+248
to
+249
| if !client.InsecureAllowWildcardRedirectURIs && strings.Contains(uri, ".*") { | ||
| return fmt.Errorf("invalid config: InsecureAllowWildcardRedirectURIs is required when using \".*\"") |
Comment on lines
+169
to
+170
| // InsecureAllowRegexpRedirectURIs is an additiona flag allowing, add to | ||
| // RedirectURIs regexp expressions for dynamic URIs. |
Comment on lines
+159
to
+170
| func surroundRedirectURIRegexp(uri string) (result string) { | ||
| result = uri | ||
| if result[0] != '^' { | ||
| result = "^" + result | ||
| } | ||
|
|
||
| if result[len(result)-1] != '$' { | ||
| result = result + "$" | ||
| } | ||
|
|
||
| return | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adding new flags for conditional logic and allowing back compatibility of the feature
Overview
Adds support for regexp-based redirect URI matching in static clients, controlled by two new flags. This addresses issue #448 and maintains full backward compatibility.
What this PR does / why we need it
New fields on storage.Client:
Behavior:
Special notes for your reviewer
The regexp matching is opt-in per client via explicit flags. The double-flag design (InsecureAllowRegexpRedirectURIs + InsecureAllowWildcardRedirectURIs) adds a deliberate friction layer so wildcards can't be used accidentally. Regexp compilation errors are caught at startup, not at auth time.